Skip to content

fix: return 400 on duplicate logger name in PUT /api/logger/:id#710

Open
aabhinavvvvvvv wants to merge 8 commits into
jlab-sensing:mainfrom
aabhinavvvvvvv:fix/issue-626-duplicate-logger-name
Open

fix: return 400 on duplicate logger name in PUT /api/logger/:id#710
aabhinavvvvvvv wants to merge 8 commits into
jlab-sensing:mainfrom
aabhinavvvvvvv:fix/issue-626-duplicate-logger-name

Conversation

@aabhinavvvvvvv
Copy link
Copy Markdown
Contributor

Summary

Fixes #626 — renaming a logger to a duplicate name caused the server to hang with no feedback shown to the user.

Changes:

  • backend/api/resources/logger.py — Duplicate name check in PUT handler, now returns 400 immediately instead of hanging.
  • frontend/src/pages/profile/components/EditLoggerModal.jsx — Show error message inline when update fails.
  • backend/api/schemas/cell_schema.py — Fix marshmallow 4 startup crash (missing User import).

Demo

ents_logger.mp4

@jmadden173
Copy link
Copy Markdown
Contributor

/deploy

@github-actions
Copy link
Copy Markdown

PR deployed to development server

Commit: 710
SHA: 48b87df

Triggered by: @jmadden173

Copy link
Copy Markdown
Contributor

@jmadden173 jmadden173 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The implementation looks good.

TODOs:

  • a unit test for the case
  • adding a line in the changelog linking to the issue.
  • Fix merge conflicts
  • Get the linter to pass

@jmadden173 jmadden173 added the awaiting-changes For PRs requiring additional changes. label Mar 21, 2026
aabhinavvvvvvv added a commit to aabhinavvvvvvv/ENTS-backend that referenced this pull request Mar 21, 2026
Also adds CHANGELOG entry for jlab-sensing#710.

Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
aabhinavvvvvvv added a commit to aabhinavvvvvvv/ENTS-backend that referenced this pull request Mar 21, 2026
@aabhinavvvvvvv aabhinavvvvvvv force-pushed the fix/issue-626-duplicate-logger-name branch from 9d4205a to ce4cd20 Compare March 21, 2026 08:34
@aabhinavvvvvvv aabhinavvvvvvv force-pushed the fix/issue-626-duplicate-logger-name branch from ce4cd20 to f1d99e9 Compare March 21, 2026 13:12
@aabhinavvvvvvv
Copy link
Copy Markdown
Contributor Author

@jmadden173 The workflow uses pull_request_target with a plain actions/checkout@v6 and this checks out the BASE branch (upstream/main), not our PR branch. So black is always running on upstream/main's code, and upstream/main has #from api.models.user import User (without space) in test_cell.py.

@aabhinavvvvvvv
Copy link
Copy Markdown
Contributor Author

@jmadden173 All our tests pass (63/64). The only failure is test_get_teros_obj_fraction_scales_to_percent which is a pre-existing bug unrelated to this PR — TEROSData.get_teros_data_obj() no longer returns a vwc_unit key that the test expects. This failure exists in jlab-sensing:main independently of our changes.

@jmadden173
Copy link
Copy Markdown
Contributor

Yes I see that, I did not realize how the pull request target was working when I applied that fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

awaiting-changes For PRs requiring additional changes.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Missing error when renaming loggers to duplicate name

2 participants